Skip to content

feat: add --inject flag to codeflash compare#1985

Merged
KRRT7 merged 2 commits intomainfrom
feat/compare-inject-flag
Apr 3, 2026
Merged

feat: add --inject flag to codeflash compare#1985
KRRT7 merged 2 commits intomainfrom
feat/compare-inject-flag

Conversation

@KRRT7
Copy link
Copy Markdown
Collaborator

@KRRT7 KRRT7 commented Apr 3, 2026

  • Add --inject flag to codeflash compare that copies files/directories from the working tree into both worktrees before benchmark discovery and execution
  • Solves the common workflow problem where benchmark files don't exist at either the base or head ref (e.g., benchmarking an already-merged optimization)

Problem

When running codeflash compare <base_ref> <head_ref>, codeflash creates git worktrees at those refs. If the benchmark test was written after both refs — the typical case when benchmarking a merged optimization — the benchmark doesn't exist in either worktree and the comparison fails with "no tests ran."

The existing benchmarks_root copy logic inside run_benchmark_on_worktree() partially addresses this, but only for the benchmarks directory as a whole and only in the memory-only path. The --inject flag provides explicit, general-purpose file injection that works for any file (benchmark tests, pyproject.toml config, conftest.py, etc.).

Usage

# Inject a benchmark file that only exists in the working tree
codeflash compare 189a6124 62dd690b --memory \
  --inject tests/benchmarks/test_benchmark_table_extraction_memory.py pyproject.toml

# Inject an entire directory
codeflash compare main feature-branch --inject tests/benchmarks/

Paths are relative to the repo root. Files are copied into the same relative path in both worktrees. Missing source paths emit a warning and are skipped.

Test plan

  • All 31 existing compare tests pass
  • End-to-end test: codeflash compare 189a6124 62dd690b --memory --inject tests/benchmarks/test_benchmark_table_extraction_memory.py pyproject.toml succeeds where it previously failed with "no tests ran"
  • Memory comparison shows expected -17% peak reduction (65.9 MiB → 55.0 MiB)

When benchmarking already-merged optimizations, the benchmark file
often doesn't exist at either the base or head ref. The --inject flag
copies specified files/directories from the working tree into both
worktrees before benchmark discovery and execution, eliminating the
need to cherry-pick benchmark commits onto temporary branches.

Usage:
  codeflash compare <base> <head> --inject tests/benchmarks/test_bench.py
@claude
Copy link
Copy Markdown
Contributor

claude bot commented Apr 3, 2026

Claude finished @KRRT7's task in 1m 58s —— View job


PR Review Summary

  • Triage PR size — SMALL (29 additions, 3 files)
  • Run lint/typecheck
  • Resolve stale threads — none found
  • Review code
  • Duplicate detection
  • Bot PR review

Prek Checks

uv run prek run --from-ref origin/mainPassed (ruff check and format both clean). No fixes needed.

Mypy errors in cmd_compare.py (ScriptCompareResult has no attribute base_stats/head_stats) are pre-existing and unrelated to this PR.


Code Review

The implementation is clean and correct. Two minor findings:

1. Inline import shutil inside conditional block (compare.py:350)

The file already has several inline import shutil statements (lines 350, 406, 457, 611) — this is an existing pattern in the file, so the new code is consistent. That said, shutil is used in 4+ places and belongs at the top-level imports. This is pre-existing tech debt and not a blocker for this PR.

2. --inject silently ignored when path is a symlink

src.exists() returns False for broken symlinks, and the is_dir() / is_file() branches don't cover symlinks. This is an edge case and probably not worth handling now, but worth noting for future robustness.

No bugs, security issues, or breaking API changes found.


Duplicate Detection

No duplicates detected. The inject logic is new and localized to compare.py. The shutil.copytree + shutil.copy2 pattern used here differs from the existing benchmarks_root copy logic (which is non-general and specific to the memory path), so there's no meaningful duplication.


Bot PRs

PR #1954 (codeflash/optimize-pr1949-2026-04-01T17.42.23) has merge conflicts but is only 2 days old — leaving open per policy (< 3 days). The optimized function TestResults.total_passed_runtime still exists.


Last updated: 2026-04-03T16:24Z

- Replace `import shutil as _shutil` with plain `import shutil` to
  match the existing style in the same function
- Warn when --inject is used with --script mode (unsupported combo)
  instead of silently dropping the flag
@KRRT7 KRRT7 merged commit 8d51e2d into main Apr 3, 2026
25 of 27 checks passed
@KRRT7 KRRT7 deleted the feat/compare-inject-flag branch April 3, 2026 16:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant